Use cursor-based pagination in iterateAllThreads#2732
Conversation
WalkthroughThe Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/threads/helpers.ts (1)
124-149:⚠️ Potential issue | 🟡 MinorHandle
total = 0before fetching.With
total = 0, the generator still fetches and yields at least one thread before returning. If callers can pass0, consider an early return.✅ Suggested fix
export async function* iterateAllThreads(total: number | undefined = undefined) { const limit = 100; let cursor: string | undefined = undefined; let count = 0; + if (total !== undefined && total <= 0) { + return; + } while (true) {
🤖 Fix all issues with AI agents
In `@src/routes/threads/helpers.ts`:
- Around line 124-138: iterateAllThreads currently paginates without a
deterministic order, causing unstable cursor pagination; update the query set in
iterateAllThreads to include an explicit ordering (e.g.,
Query.orderAsc('$sequence') for monotonic insertion order) before calling
databases.listDocuments, ensuring the same ordering is applied when you pass a
cursor so pages are stable and no duplicates/skip occur; also apply the same
explicit order change to the other pagination site in this file (compare with
getThreads which uses Query.orderDesc('$createdAt')) so both methods use a
consistent, deterministic order.
What does this PR do?
Moving to cursor pagination when fetching all threads.
Have you read the Contributing Guidelines on issues?
✅
Summary by CodeRabbit